8378908: [lworld] Serializations warnings for value classes that have private serialization methods by vicente-romero-oracle · Pull Request #2187 · openjdk/valhalla

vicente-romero-oracle · 2026-03-02T20:50:19Z

This PR is adding a new serialization warning for the case when a value class declares one of the following serialization related methods:

readObject
writeObject
readObjectNoData

They have no effect if declared in value classes, fact that should be notified to the user,

TIA

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JDK-8378908: [lworld] Serializations warnings for value classes that have private serialization methods (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2187/head:pull/2187
$ git checkout pull/2187

Update a local copy of the PR:
$ git checkout pull/2187
$ git pull https://git.openjdk.org/valhalla.git pull/2187/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2187

View PR using the GUI difftool:
$ git pr show -t 2187

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2187.diff

Using Webrev

Link to Webrev Comment

… private serialization methods

vicente-romero-oracle · 2026-03-02T21:50:28Z

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java

-                            case "readObject"       -> checkReadObject(tree,e, method);
-                            case "readObjectNoData" -> checkReadObjectNoData(tree, e, method);
-                            case "readResolve"      -> checkReadResolve(tree, e, method);
+                            case "writeObject"      -> isSerialMethodCorrect[0] = hasAppropriateWriteObject(tree, e, method);


some changes in this method and in the methods invoked here were not strictly necessary but were done for the sake of uniformity

Can we convert this to a switch expression, like:

isSerialMethodCorrect[0] = switch (name) { case "writeObject" -> hasAppropriateWriteObject(tree, e, method);

and so on.

bridgekeeper · 2026-03-03T09:32:34Z

👋 Welcome back vromero! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-03-03T09:41:51Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

mlbridge · 2026-03-03T09:52:19Z

Webrevs

liach · 2026-03-04T03:54:28Z

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java

-                            case "readObject"       -> checkReadObject(tree,e, method);
-                            case "readObjectNoData" -> checkReadObjectNoData(tree, e, method);
-                            case "readResolve"      -> checkReadResolve(tree, e, method);
+                            case "writeObject"      -> isSerialMethodCorrect[0] = hasAppropriateWriteObject(tree, e, method);


Can we convert this to a switch expression, like:

isSerialMethodCorrect[0] = switch (name) { case "writeObject" -> hasAppropriateWriteObject(tree, e, method);

and so on.

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java

…java Co-authored-by: Chen Liang <liach@openjdk.org>

jddarcy · 2026-03-04T16:57:34Z

src/java.base/share/classes/java/time/chrono/HijrahDate.java

     * @throws InvalidObjectException always
     */
    @java.io.Serial
+    @SuppressWarnings("serial")


The other uses of @SuppressSerial in the JDK tend to have a comment explaining why the warning is being suppressed.

jddarcy · 2026-03-04T16:58:50Z

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java

            // fields
-            final boolean[] hasWriteReplace = {false};
+            final Map<String, Symbol> declaredSerialMethodNames = new HashMap<>();
+            final boolean[] isSerialMethodCorrect = new boolean[] { false };


Why is a one-element boolean array being used as opposed to a (non-final) boolean?

Because this is passed to a lambda expression, which requires final local variables.

jddarcy · 2026-03-04T16:59:33Z

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java

                                    LintWarnings.SerializableValueClassWithoutWriteReplace2);
                }
            }
+            if (c.isValueClass()) {


I think a short comment here explaining the rules for serializable value classes would be helpful.

jddarcy · 2026-03-04T17:00:31Z

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java


            // Check for missing serialVersionUID; check *not* done
            // for enums or records.
            VarSymbol svuidSym = null;


Do value classes need any distinct serialVersionUID handling?

not sure I get your question. Why should they?

Hi @RogerRiggs, thanks for making a comment in an OpenJDK project!

All comments and discussions in the OpenJDK Community must be made available under the OpenJDK Terms of Use. If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please Use "Add GitHub user RogerRiggs" for the summary.

If you are not an OpenJDK Author, Committer or Reviewer, simply check the box below to accept the OpenJDK Terms of Use for your comments.

I agree to the OpenJDK Terms of Use for all comments I make in a project in the OpenJDK GitHub organization.

Your comment will be automatically restored once you have accepted the OpenJDK Terms of Use.

not sure I get your question. Why should they?

I don't know if they do; that's why I was asking :-)

I think my concerns would be addressed by a "here's how serial value classes work."

Oh I see, thanks @RogerRiggs for your answer. I didn't know that specific about migrated value classes. I will add a comment

Hi @RogerRiggs, thanks for making a comment in an OpenJDK project!

All comments and discussions in the OpenJDK Community must be made available under the OpenJDK Terms of Use. If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please Use "Add GitHub user RogerRiggs" for the summary.

If you are not an OpenJDK Author, Committer or Reviewer, simply check the box below to accept the OpenJDK Terms of Use for your comments.

I agree to the OpenJDK Terms of Use for all comments I make in a project in the OpenJDK GitHub organization.

Your comment will be automatically restored once you have accepted the OpenJDK Terms of Use.

jddarcy · 2026-03-12T04:43:12Z

@vicente-romero-oracle, updates generally looks good; I can given a final re-review after JavaOne.

8378908: [lworld] Serializations warnings for value classes that have…

48c47e3

… private serialization methods

vicente-romero-oracle marked this pull request as draft March 2, 2026 20:58

adding tests plus bug fix

f539950

vicente-romero-oracle marked this pull request as ready for review March 2, 2026 21:47

vicente-romero-oracle commented Mar 2, 2026

View reviewed changes

openjdk bot added the rfr Pull request is ready for review label Mar 3, 2026

liach reviewed Mar 4, 2026

View reviewed changes

vicente-romero-oracle and others added 2 commits March 4, 2026 06:52

Update src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.…

acbf6b3

…java Co-authored-by: Chen Liang <liach@openjdk.org>

Update src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.…

03d9776

…java Co-authored-by: Chen Liang <liach@openjdk.org>

jddarcy reviewed Mar 4, 2026

View reviewed changes

vicente-romero-oracle added 3 commits March 5, 2026 18:37

Merge branch 'lworld' into JDK-8378908

e22a52f

Merge branch 'lworld' into JDK-8378908

e3ca779

addressing review comments

936d024

vicente-romero-oracle requested a review from jddarcy March 6, 2026 18:12

doc change

8a539cd

Conversation

vicente-romero-oracle commented Mar 2, 2026 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bridgekeeper bot commented Mar 3, 2026

Uh oh!

openjdk bot commented Mar 3, 2026

Uh oh!

mlbridge bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RogerRiggs Mar 6, 2026 • edited by bridgekeeper bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vicente-romero-oracle Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RogerRiggs Mar 6, 2026 • edited by bridgekeeper bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jddarcy commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

vicente-romero-oracle commented Mar 2, 2026 •

edited by openjdk bot

Loading

mlbridge bot commented Mar 3, 2026 •

edited

Loading

RogerRiggs Mar 6, 2026 •

edited by bridgekeeper bot

Loading

vicente-romero-oracle Mar 6, 2026 •

edited

Loading

RogerRiggs Mar 6, 2026 •

edited by bridgekeeper bot

Loading